build: drop CMake presets, move the wheel tree out of build/ - #913
build: drop CMake presets, move the wheel tree out of build/#913jiwenc-nv wants to merge 1 commit into
Conversation
|
📝 Docs preview is not auto-deployed for fork PRs. A maintainer with write access to |
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughThe change separates managed CMake and wheel build outputs into Estimated code review effort: 2 (Simple) | ~10 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
55c94e9 to
64cc93b
Compare
The presets existed to give each Python version its own directory, because the interpreter and ABI are baked into a build tree. That bought isolation at the cost of making `cmake -B build` — the form every other page and every plugin still used — the unsupported path, and it put the managed trees underneath build/ so `rm -rf build` took out all of them at once. Go back to `cmake -B build` as the one documented flow, and enforce the isolation directly instead: ISAAC_TELEOP_PYTHON_VERSION is now stamped into the cache, and changing it on an existing tree is a hard error pointing at a fresh directory. Previously it was silently ignored — the cache and the "Configuring for Python X" message would say 3.12 while the extensions compiled against the 3.11 venv. The scikit-build-core tree moves to build-wheel/<cache-tag>, out of build/, so pip and a classic configure cannot collide. Python bindings stay on by default. Signed-off-by: Jiwen Cai <jiwenc@nvidia.com>
64cc93b to
6666bdc
Compare
|
/preview-docs |
|
✅ Preview deployed: https://NVIDIA.github.io/IsaacTeleop/preview/pr-913/ |
|
Superseded by #916, which carries the same change from a branch on NVIDIA/IsaacTeleop rather than a personal fork. |
The presets existed to give each Python version its own build directory, since the interpreter and ABI are baked into a build tree. That isolation came at a price:
cmake -B build— the form every other page and every plugin still used — became the unsupported path, and the managed trees sat underneathbuild/, sorm -rf buildtook out all of them at once.This goes back to
cmake -B buildas the single documented flow and enforces the isolation directly instead.ISAAC_TELEOP_PYTHON_VERSIONis stamped into the cache, and changing it on an existing tree is now a hard error naming a fresh directory to use.That guard is the one addition beyond the mechanical sweep, and it closes a real silent bug: passing
-DISAAC_TELEOP_PYTHON_VERSION=3.12to a tree configured for 3.11 previously printed "Configuring for Python 3.12" and cached 3.12 while still compiling against the 3.11 venv. Happy to drop it if you'd rather just document "use a fresh directory".The scikit-build-core tree moves to
build-wheel/<cache-tag>, outsidebuild/, so pip and a classic configure can't collide. Python bindings stay ON by default.CI keeps its 3-Python matrix via
-DISAAC_TELEOP_PYTHON_VERSION=; only GitHub-hosted runners build, so a plainbuild/is safe.Testing
On Ubuntu 24.04 aarch64:
cmake -B build→ build →cmake --install: green, 0 errors, produceswheels/isaacteleop-1.5+local-cp311-cp311-linux_aarch64.whl.TRUEare adopted silently rather than erroring — verified aTRUE-stamped cache reconfigures clean and re-stamps to3.11.sphinx -Wbuilds clean.